-
Notifications
You must be signed in to change notification settings - Fork 47
Update the maximum line width in .clang-format #317
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Drew Lewis <[email protected]>
|
@jjcherry56 if you'll explain a bit about what is failing in clang-format for you I think we might be able to fix it to do what you want. I wasn't 100% sure what wasn't working for you based on the comments in the file. |
|
If you run clang-format and look at the diffs you can see all the places it fails. |
Signed-off-by: Drew Lewis <[email protected]>
2a39a60 to
8e697b0
Compare
|
@jjcherry56 I made some changes that fix at least some of your issues and I tried it on Search.{hh,cc} and Bfs.cc. Is there still stuff you don't like? |
|
I don't have time to deal with this right now |
|
Would you be willing to let me own the .clang-format then. You still make formatting choices WRT PRs and the .clang-format won't be the source of truth, but I can try to make the clang-format as close as possible for other contributors to make it easier for them? |
| BfsIterator(bfs_index, level_max, 0, search_pred, sta) | ||
| SearchPred *search_pred, | ||
| StaState *sta) : | ||
| BfsIterator(bfs_index, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could live with this but I don't understand why these args are on separate lines when other function calls are broken by line width.
| Level level_max, | ||
| SearchPred *search_pred, | ||
| StaState *sta) : | ||
| Level level_min, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with untabification, but the problem is I have some branches with very extensive changes that will make merge/rebase next to impossible unless I can come up with a strategy to do them both at the same time so there are no conflicts.
| BfsIterator::~BfsIterator() | ||
| { | ||
| } | ||
| BfsIterator::~BfsIterator() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weirdness
| && search_pred->searchFrom(from_vertex) | ||
| && search_pred->searchThru(edge)) | ||
| enqueue(from_vertex); | ||
| if (from_vertex->level() >= to_level && search_pred->searchFrom(from_vertex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like having the boolean clauses broken by lines, but it may not be smart enough to do that.
Update .clang-format based on comments in #316